feat: use issuer url instead of well known url#1371
feat: use issuer url instead of well known url#1371ZohebShaikh wants to merge 8 commits intomainfrom
Conversation
ZohebShaikh
changed the title
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1371 +/- ##
==========================================
- Coverage 95.30% 95.29% -0.02%
==========================================
Files 43 43
Lines 3198 3212 +14
==========================================
+ Hits 3048 3061 +13
- Misses 150 151 +1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
ZohebShaikh
changed the title
tpoliaw
left a comment
tpoliaw
left a comment
There was a problem hiding this comment.
I like this change - it makes the auth config much clearer, thanks.
That said, this is a breaking change that will need every beamline to update its config and for GDA to support the new API. We have a list of breaking issues that are waiting for a 2.0 release so it might be worth adding an issue there for this change and keeping this PR around until we decide to break everything at once.
| def _config_from_oidc_url(self) -> dict[str, Any]: | ||
| response: requests.Response = requests.get(self.well_known_url) | ||
| response: requests.Response = requests.get( | ||
| self.issuer + "/.well-known/openid-configuration" |
There was a problem hiding this comment.
May be worth calling out that this address must exist, that may determine what the "issuer" is.
There was a problem hiding this comment.
Spec for this is here as a MUST so we can probably rely on it.
OpenID Providers supporting Discovery MUST make a JSON document available at the path formed by concatenating the string /.well-known/openid-configuration to the Issuer
|
@tpoliaw I have made it backwards compatible so in v2 we can remove the backwards compatibility. |
3 participants
No description provided.